feat: add automatic low-power preference - #2995
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 90dcf06342
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
clawsweeper
Bot
added
P3
|
Codex review: needs maintainer review before merge. Reviewed August 17, 2026, 7:36 AM ET / 11:36 UTC. ClawSweeper reviewWhat this changesThis PR replaces the background-refresh low-power toggle with Off, On, and Automatic choices that can follow macOS Low Power Mode. Merge readinessKeep open for a maintainer product-direction decision on adding a persistent Automatic low-power choice; no concrete patch defect was found. Likely related people: Carl (original low-power feature) and Peter Steinberger (recent default-branch owner). Priority: P3 Review scores
Verification
How this fits togetherCodexBar’s Settings Store reads a persisted preference and exposes an effective low-power boolean to background refresh work. That value controls refresh scheduling, storage scans, and related battery-saving behavior. flowchart LR
A[Settings picker] --> B[Stored low-power preference]
C[macOS Low Power Mode] --> D[Effective low-power decision]
B --> D
D --> E[Background refresh timer]
D --> F[Storage and provider refreshes]
E --> G[Usage updates]
Decision needed
Why: The implementation is coherent and preserves existing stored values, but whether this new user-facing mode belongs in the supported settings surface is a maintainer product choice. Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: If the product direction is approved, land the three-state preference with its compatibility migration and power-state timer restart; otherwise retain the existing explicit toggle. Do we have a high-confidence way to reproduce the issue? Not applicable: this PR adds a settings capability rather than reporting broken existing behavior; its attached screenshot directly shows the new picker. Is this the best way to solve the issue? Yes, conditionally: a three-state persisted preference with a narrow legacy migration is a maintainable implementation, but accepting the new mode requires product approval. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 7c64d280f34f. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (1 earlier review cycle)
|
2 participants
Summary
Off/On/Automaticdropdown (sameSettingsMenuPickercomponent used by Refresh Interval)LowPowerModePreferenceenum;Automaticresolves against the live system Low Power Mode state (ProcessInfo.processInfo.isLowPowerModeEnabled)backgroundWorkLowPowerModeEnabled(read by every throttling call site: adaptive refresh, fixed-timer refresh, storage refresh, reset-boundary catch-up, Codex cost catch-up, iCloud sync gating) becomes a computed property derived from the new preference, so none of those call sites needed to changeUserDefaultsbool to the new preference on first load, preserving existing users' behavior exactly (true→on,false/missing →off)NSProcessInfoPowerStateDidChangeinSettingsStoreand bumpbackgroundWorkSettingsRevisionwhenever the preference is.automatic, so a running fixed-frequency refresh timer reclamps/unclamps immediately when the user toggles system Low Power Mode mid-session instead of waiting for an unrelated settings change to restart itOff/On/Automaticoption labels reuse existing keys already translated in all 23 localesTests
background low power mode migrates legacy enabled flag to on preferencetoSettingsStoreCoverageTests— verifies the legacy bool key migrates correctly and the new preference key persistsbackground low power mode defaults off persists and drives effective web saverto exercise the newbackgroundWorkLowPowerModePreferencepropertyAdaptiveRefreshHeuristicsTests,AdaptiveRefreshTimerTests,CodexBackgroundRefreshCoalescingTests,ProviderStorageFootprintTests) that force-enabled the old bool to set.backgroundWorkLowPowerModePreference = .oninsteadswift testsuite for the touched files green; twoAdaptiveRefreshTimerTeststimer-timeout failures are pre-existing flakes unrelated to this change (confirmed identical failures on unmodified base branch)ProviderArchitectureGatekeeperTestsallowlist: the newLowPowerModePreferencecode shifted the factory/minimax/zai construct inSettingsStore.swiftfrom line 1073 to 1126; updated the pinned line number (anchor/fingerprint unchanged) to unblock theswift-test-macosCI shardVerification
swift buildcleanmake checkclean (0 lint violations, 0 format changes)Screenshots